Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glsl override for nouveau #681

Draft
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

pijaro
Copy link
Contributor

@pijaro pijaro commented May 6, 2021

There is an issue with Nouveau drivers with glsl150 shaders (introduced in PR #668).

It looks like Nouveau can't use GLSL 1.50 programs, even if the driver reports that it supports even newer version of GLSL.

Proposed piece of code is a workaround for Nouveau driver users in linux. It sets the rviz glsl version to 1.20 which works fine (but is slower than 1.50).

pijaro added 2 commits May 6, 2021 17:54
Signed-off-by: Piotr Jaroszek <piotr.jaroszek@robotec.ai>
Signed-off-by: Piotr Jaroszek <piotr.jaroszek@robotec.ai>
@pijaro
Copy link
Contributor Author

pijaro commented May 10, 2021

Looks like the Intel drivers (for integrated GPU) are also affected.

The question rises, are nvidia drivers working as they intended to (GL 4.6 with GLSL>=150) and the other drivers are somewhat misleading (GL 4.6 but without GLSL>=150) - or - nvidia do what it shouldn't be able to and other drivers are actually working correctly?

I'm still looking into it.

Signed-off-by: Piotr Jaroszek <piotr.jaroszek@robotec.ai>
@clalancette
Copy link
Contributor

Since this is still being looked into, I'm actually going to convert it to a draft. Feel free to convert back when it is ready.

@clalancette clalancette marked this pull request as draft June 3, 2021 13:12

#ifdef __linux__
// TODO(pijaro): Nouveanu and Intel drivers don't support glsl150 even if they "should".
// We fix version to 120 since this is the one we are currently using in materials.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pijaro Sorry, this PR dropped off my radar.

It sounds like it would be good to merge this workaround until we resolve the core issue.
Could you reference the open ticket here in this comment? #691

@audrow audrow changed the base branch from ros2 to rolling June 28, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants